chore: add related values #6619
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
srikanthccv
force-pushed
the
add-related-values
branch
from
b935c5c to
ce78d4f
Compare
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
❌ Changes requested. Reviewed everything up to 45b423e in 2 minutes and 9 seconds
More details
- Looked at
372lines of code in5files - Skipped
0files when reviewing. - Skipped posting
9drafted comments based on config settings.
1. pkg/query-service/app/parser.go:377
- Draft comment:
Consider replacing the literal 30000000000 with a named constant (e.g. bufferDuration) to clarify that it represents 30 seconds in nanoseconds. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
2. pkg/query-service/model/v3/v3.go:995
- Draft comment:
There is commented-out validation code for time aggregation (lines 995-1007). If it's no longer needed, please remove it; otherwise add a comment explaining why it’s retained. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
3. pkg/query-service/model/v3/v3.go:1129
- Draft comment:
In CompositeQuery.Clone, deep cloning of slices (e.g. for BuilderQueries, ClickHouseQueries, PromQueries) is handled but GroupBy (if ever mutable) may need cloning for safety. Verify whether a shallow copy suffices. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
4. pkg/query-service/app/parser.go:784
- Draft comment:
Consider replacing the numeric time offsets (subtracting 6*2 hours from StartTimeMillis and adding 6 hours to EndTimeMillis) with a named constant for clarity. - Reason this comment was not posted:
Marked as duplicate.
5. pkg/query-service/app/parser.go:810
- Draft comment:
The variable validation in validateExpressions() loops over BuilderQueries comparing each query’s Expression with each variable extracted from the expression. This approach may be brittle if variable names differ from query expressions. Consider validating against an explicit set of expected variable names (e.g. from the Variables map). - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
6. pkg/query-service/app/parser.go:367
- Draft comment:
A magic constant (30000000000) is used to adjust the time in parseTimeMinusBufferStr(). Define a named constant (e.g. bufferDuration) to improve readability and maintainability. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
7. pkg/query-service/model/v3/v3.go:433
- Draft comment:
The Clone method for QueryRangeParamsV3 shallow-copies the Variables map. If the map is mutable and may be updated later, consider performing a deep copy to avoid unintended sharing. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
8. pkg/query-service/model/v3/v3.go:1363
- Draft comment:
In Point's MarshalJSON and UnmarshalJSON methods, the numeric value is encoded as a string. Ensure that encoding point values as strings is intentional, as downstream consumers might expect numeric JSON values. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
9. pkg/query-service/model/v3/v3.go:995
- Draft comment:
There is commented-out code in BuilderQuery.Validate (lines ~995–1012) concerning aggregate operator and time aggregation. If it is no longer needed, please remove it to clean up the code. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
Workflow ID: wflow_VllatKRRhWjPgch2
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 2cb249d in 1 minute and 9 seconds
More details
- Looked at
62lines of code in2files - Skipped
0files when reviewing. - Skipped posting
8drafted comments based on config settings.
1. pkg/query-service/model/v3/v3.go:310
- Draft comment:
Renamed field from selectedAttributeValues to existingFilterItems. Ensure clients consuming the API are updated. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
This comment is asking the PR author to ensure that clients consuming the API are updated, which violates the rule against asking the author to ensure behavior is intended or tested. It doesn't provide a specific code suggestion or ask for a specific test to be written.
2. pkg/query-service/model/v3/v3.go:337
- Draft comment:
Error message now refers to existingFilterItems instead of selectedAttributeValues. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
This comment is purely informative and does not provide any actionable feedback or suggestions for improvement. It simply states a change that has been made without indicating any potential issues or areas for improvement.
3. pkg/query-service/model/v3/v3.go:419
- Draft comment:
New field 'RelatedValues' added. Consider documenting its intended use and potential recursion limits. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
4. pkg/query-service/model/v3/v3.go:990
- Draft comment:
Remove or clean up the commented‐out validation block in BuilderQuery.Validate to improve code clarity. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
5. pkg/query-service/model/v3/v3.go:271
- Draft comment:
AttributeKeyDataType.Validate accepts only primitive types; consider whether array types should be allowed. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
6. pkg/query-service/model/v3/v3.go:920
- Draft comment:
BuilderQuery.Clone shallow-copies slices (e.g. GroupBy and Functions). Verify that mutable maps (e.g. NamedArgs inside Functions) are safe from unintended mutations. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
7. pkg/query-service/model/v3/v3.go:4570
- Draft comment:
GetTimeSeriesResultV3 uses reflection to allocate scan targets; consider optimizing this for high‐performance queries. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
8. pkg/query-service/model/v3/v3.go:1363
- Draft comment:
Point.MarshalJSON converts the numeric value to a string; ensure consumers expecting a numeric type are properly handled. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
Workflow ID: wflow_Na17JvqDiIFwC6IU
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
nityanandagohain
approved these changes
Feb 6, 2025
vikrantgupta25
added a commit
that referenced
this pull request
Feb 7, 2025
This reverts commit c5219ac.
nityanandagohain
pushed a commit
that referenced
this pull request
Feb 7, 2025
SagarRajput-7
pushed a commit
that referenced
this pull request
Feb 10, 2025
SagarRajput-7
pushed a commit
that referenced
this pull request
Feb 10, 2025
hudsongeovane
pushed a commit
to hudsongeovane/signoz
that referenced
this pull request
Feb 22, 2025
hudsongeovane
pushed a commit
to hudsongeovane/signoz
that referenced
this pull request
Feb 22, 2025
This reverts commit c5219ac.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Part of #6934
Example:
request payload
{ "startTimeMillis": 1738776047000, "endTimeMillis": 1738777847000, "aggregateOperator": "noop", "dataSource": "logs", "aggregateAttribute": "", "filterAttributeKey": "k8s.pod.name", "searchText": "", "filterAttributeKeyDataType": "string", "tagType": "resource", "selectedAttributeValues": [ { "key": { "key": "k8s.namespace.name", "dataType": "string", "type": "resource" }, "op": "IN", "value": [ "nightswatch" ] } ] }The
selectedAttributeValuespart sends the selected value of other attributes that exist. In the above request payload, the namespace selected value is sent. In the response, we send pods from all namespaces and also related pod names from existing selection. On the UI, we surface the related values at the top before showing all the values.The response
{ "status": "success", "data": { "stringAttributeValues": [ <redacted> ], "numberAttributeValues": null, "boolAttributeValues": null, "relatedValues": { "stringAttributeValues": [ "", "signoz-otel-collector-logs-7586bbb4d9-7wgl6", "signoz-otel-collector-traces-78f6d754db-l6fvb", "signoz-otel-collector-metrics-7745b9b696-nznlg", "app-signoz-zookeeper-0", "chi-app-signoz-clickhouse-cluster-2-0-0", "signoz-otel-collector-metrics-7745b9b696-lmljl", "signoz-query-service-0", "signoz-otel-collector-metrics-7745b9b696-49f4h", "signoz-otel-collector-metrics-7745b9b696-7nbsc", "signoz-otel-collector-metrics-7745b9b696-tn62p", "signoz-otel-collector-metrics-7745b9b696-fqggk", "signoz-otel-collector-logs-7586bbb4d9-xxwng", "chi-app-signoz-clickhouse-cluster-0-0-0", "signoz-otel-collector-metrics-7745b9b696-kbqsb", "signoz-otel-collector-metrics-7745b9b696-b8kjh", "signoz-otel-collector-logs-7586bbb4d9-wqk8c", "signoz-otel-collector-traces-78f6d754db-s9tz8", "app-signoz-clickhouse-operator-85d46978b9-n96gk", "signoz-otel-collector-traces-78f6d754db-44w68", "signoz-otel-collector-traces-78f6d754db-pc8pp", "signoz-otel-collector-metrics-7745b9b696-99mp6", "signoz-otel-collector-traces-78f6d754db-9cm9n", "signoz-otel-collector-metrics-7745b9b696-tb9dp", "chi-app-signoz-clickhouse-cluster-1-0-0", "signoz-otel-collector-traces-78f6d754db-wtrc4", "signoz-otel-collector-traces-78f6d754db-8l8r7", "signoz-alertmanager-0" ], "numberAttributeValues": null, "boolAttributeValues": null, "relatedValues": null } } }Important
Adds support for fetching related attribute values in query service, including new request handling and database query logic.
FetchRelatedValues()inreader.goto fetch related attribute values based on existing filters.GetLogAttributeValues()andGetTraceAttributeValues()inreader.goto include related values in the response./autocomplete/attribute_valuesinhttp_handler.goto handle related values.StartTimeMillisandEndTimeMillistoFilterAttributeValueRequestinv3.go.RelatedValuesfield toFilterAttributeValueResponseinv3.go.Validate()method inFilterAttributeValueRequestinv3.go.parseFilterAttributeValueRequestBody()inparser.goto parse POST request body for attribute values.defaultMetadataDBanddefaultMetadataTableinoptions.go.This description was created by
for 2cb249d. It will automatically update as commits are pushed.